Add asc keyword to sort command by ritvibhatt · Pull Request #4113 · opensearch-project/sql

ritvibhatt · 2025-08-22T22:37:39Z

Description

Adds asc/a keyword as option in sort command, Specifying asc does not change behavior, sort direction of each field remains as specified (ascending as default)
Adds missing tests to CaclitePPlSortIT for desc keyword, count, and type casting enhancements

Changes

Added ASC and A tokens to PPL lexer and parser grammar
Updated sort command to accept asc/a keywords alongside existing desc/d

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Swiddis

Needs doc, otherwise LGTM

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

RyanL1997

Hi @ritvibhatt, thanks for taking this on, and i just left some comments.

RyanL1997 · 2025-08-28T17:31:19Z

docs/user/ppl/cmd/sort.rst

 * [+|-]: optional. The plus [+] stands for ascending order and NULL/MISSING first and a minus [-] stands for descending order and NULL/MISSING last. **Default:** ascending order and NULL/MISSING first.
 * sort-field: mandatory. The field used to sort. Can use ``auto(field)``, ``str(field)``, ``ip(field)``, or ``num(field)`` to specify how to interpret field values.
-* [desc|d]: optional. Reverses the sort results. If multiple fields are specified, reverses order of the first field then for all duplicate values of the first field, reverses the order of the values of the second field and so on.
+* [asc|a|desc|d] (Since 3.3): optional. asc/a keeps the sort order as specified. desc/d reverses the sort results. If multiple fields are specified with desc/d, reverses order of the first field then for all duplicate values of the first field, reverses the order of the values of the second field and so on. **Default:** asc.


nit: Consider rewording to 'sorts in ascending order' for clarity.

The behavior for asc is it will keep whatever the specified field sort order was, so for sort firstname, -lastname asc firstname will still be sorted ascending and lastname will still be sorted descending. Don't know if 'sorts in ascending order' would make that behavior clear

RyanL1997 · 2025-08-28T17:31:35Z

docs/user/ppl/cmd/sort.rst



-* count: optional. The number of results to return. **Default:** returns all results. Specifying a count of 0 or less than 0 also returns all results.
+* count (Since 3.3): optional. The number of results to return. **Default:** returns all results. Specifying a count of 0 or less than 0 also returns all results.


Do we have these covered in example section?

Yes! just added 'Since 3.3' in this PR but added examples in previous PR

RyanL1997 · 2025-08-28T17:32:49Z

ppl/src/main/antlr/OpenSearchPPLParser.g4


 sortCommand
-   : SORT (count = integerLiteral)? sortbyClause (DESC | D)?
+   : SORT (count = integerLiteral)? sortbyClause (ASC | A | DESC | D)?


Should we consider the precedence if someone accidentally specifies both asc and desc? The grammar allows it but behavior might be undefined.

Just checked using both asc and desc will result in an error

got it. Thanks for confirming that.

RyanL1997 · 2025-08-28T17:34:19Z

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLSortIT.java

  }
+
+  @Test
+  public void testSortWithAscKeyword() throws IOException {


nit: Consider adding negative test cases:

Invalid combinations like 'sort field asc desc'

Edge cases with count=0"

* add asc keyword to sort Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * add tests and update docs Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * remove a Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * add a keyword Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * update doc Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * empty Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * fix formatting Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> * empty Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> --------- Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> (cherry picked from commit 6db8d80) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add asc keyword to sort * add tests and update docs * remove a * add a keyword * update doc * empty * fix formatting * empty --------- (cherry picked from commit 6db8d80) Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Swiddis reviewed Aug 26, 2025

View reviewed changes

ritvibhatt marked this pull request as ready for review August 26, 2025 20:59

Swiddis previously approved these changes Aug 26, 2025

View reviewed changes

Swiddis added enhancement New feature or request backport 2.19-dev labels Aug 26, 2025

ritvibhatt force-pushed the add-asc-to-sort branch from c03f1ef to 4c67f91 Compare August 27, 2025 17:44

ritvibhatt dismissed Swiddis’s stale review via 64d5d79 August 27, 2025 18:24

ritvibhatt added 2 commits August 28, 2025 09:02

add asc keyword to sort

6a161fd

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

add tests and update docs

1c27280

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

ritvibhatt added 6 commits August 28, 2025 09:02

remove a

96cda7d

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

add a keyword

9f5eae5

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

update doc

7dcd9cf

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

empty

617090c

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

fix formatting

787621b

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

empty

6c54b41

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

ritvibhatt force-pushed the add-asc-to-sort branch from 9fe664f to 6c54b41 Compare August 28, 2025 16:02

RyanL1997 reviewed Aug 28, 2025

View reviewed changes

penghuo approved these changes Sep 2, 2025

View reviewed changes

RyanL1997 approved these changes Sep 2, 2025

View reviewed changes

Swiddis approved these changes Sep 2, 2025

View reviewed changes

Swiddis merged commit 6db8d80 into opensearch-project:main Sep 2, 2025
23 checks passed

opensearch-trigger-bot bot mentioned this pull request Sep 2, 2025

[Backport 2.19-dev] Add asc keyword to sort command #4202

Merged

Swiddis mentioned this pull request Sep 11, 2025

Audit field availability for new keywords added with the 3.3 improvements #4280

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add asc keyword to sort command#4113

Add asc keyword to sort command#4113
Swiddis merged 8 commits intoopensearch-project:mainfrom
ritvibhatt:add-asc-to-sort

ritvibhatt commented Aug 22, 2025

Uh oh!

Swiddis left a comment

Uh oh!

RyanL1997 left a comment

Uh oh!

RyanL1997 Aug 28, 2025

Uh oh!

ritvibhatt Aug 28, 2025

Uh oh!

RyanL1997 Aug 28, 2025

Uh oh!

ritvibhatt Aug 28, 2025

Uh oh!

RyanL1997 Aug 28, 2025

Uh oh!

ritvibhatt Aug 28, 2025

Uh oh!

RyanL1997 Aug 28, 2025

Uh oh!

RyanL1997 Aug 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants



		* count: optional. The number of results to return. Default: returns all results. Specifying a count of 0 or less than 0 also returns all results.
		* count (Since 3.3): optional. The number of results to return. Default: returns all results. Specifying a count of 0 or less than 0 also returns all results.

Conversation

ritvibhatt commented Aug 22, 2025

Description

Changes

Related Issues

Check List

Uh oh!

Swiddis left a comment

Choose a reason for hiding this comment

Uh oh!

RyanL1997 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants